Conversation
5a10772 to
c1603bb
Compare
|
|
I am not planning to review these PRs until they are marked as non-draft and there is a JIRA issue. |
bvolpato
changed the title
| intervals.addAll(extracted); | ||
| if (extracted == null) { | ||
| // Every disjunct must be represented to avoid excluding matching rows. | ||
| return null; |
There was a problem hiding this comment.
This function has no javadoc, so it's hard to guess what "null" means.
It's hard even to guess what the returned result is supposed to be.
Maybe you can improve it a bit?
| f.rexBuilder.makeCall(SqlStdOperatorTable.EXTRACT, | ||
| f.rexBuilder.makeFlag(TimeUnitRange.DAY), timestamp), | ||
| f.rexBuilder.makeExactLiteral(BigDecimal.valueOf(15)))); | ||
| planner.setRoot(LogicalFilter.create(query, condition)); |
There was a problem hiding this comment.
inferring the generated code is not easy, maybe you can help by adding some comments showing the equivalent SQL for the various expressions generated
| final Fixture2 f = new Fixture2(); | ||
| final RexNode range = f.lt(f.ts, f.timestampLiteral(2020, Calendar.JANUARY, 1)); | ||
| final RexNode extract = f.eq(f.exDay, f.literal(15)); | ||
| assertThat(DruidDateTimeUtils.createInterval(f.or(range, extract)), nullValue()); |
There was a problem hiding this comment.
createInterval also does not say what a "null" result means, maybe you can improve its documentation
2 participants
Jira Link
CALCITE-7818
Changes Proposed
Druid interval extraction currently ignores OR branches it cannot represent as timestamp intervals. The filter rule then treats the remaining partial intervals as the complete condition and removes the filter, excluding valid rows.
Abort interval extraction if any OR branch is unrepresentable. This lets the existing filter path retain the full predicate. Add regressions for both operand orders, an empty-range branch, and planner behavior that preserves the original intervals and filter.
Reproduction
For a Druid table whose time column is
timestamp, this predicate must retain a row dated2021-01-15:The affected plan restricts intervals to dates before 2020 and drops the filter.
Validation
./gradlew autostyleApplyandgit diff --checkpass.mainwas blocked byjava.io.IOException: No space left on device; the full Gradle build has not been validated locally.Downsides
Mixed OR predicates may scan a wider interval and rely on Druid filtering. Narrowing the interval safely requires accounting for every disjunct.